Client for reading Fastly dictionary ab test state #14344
Conversation
Co-authored-by: Imogen Hardy <[email protected]>
Jakeii
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Jakeii
changed the title
Jakeii
force-pushed
the
jlk/beta-ab-testing-client
branch
from
098ff9a to
c77cfc8
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new beta A/B testing client framework for reading Fastly dictionary A/B test state, designed to replace the current @guardian/ab-core framework. The implementation supports both server-side and client-side A/B testing with separate data sources and provides a React hook for DCR usage.
- Adds
BetaABTestsclass for managing A/B test participation with server-side and client-side support - Implements
useBetaABReact hook for accessing A/B test state in components - Integrates client-side A/B test reading from cookies and server-side tests from configuration
Reviewed Changes
Copilot reviewed 30 out of 61 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/experiments/lib/beta-ab-tests.ts |
Core beta A/B testing class implementation with server/client-side logic |
src/client/abTesting.ts |
Client-side A/B test initialization and cookie parsing logic |
src/lib/useAB.ts |
React hooks for beta A/B testing integration |
src/components/SetABTests.importable.tsx |
Integration of beta A/B tests into existing setup flow |
src/types/config.ts |
Type definition updates for new serverSideABTests property |
| Multiple schema/render files | Addition of serverSideABTests configuration throughout the application |
dotcom-rendering/src/lib/useAB.ts
Outdated
| export const useBetaAB = (): BetaABTestAPI | undefined => { | ||
| const { data } = useSWRImmutable( | ||
| 'beta-ab-tests', | ||
| () => new Promise<BetaABTestAPI>(() => {}), |
There was a problem hiding this comment.
This promise never resolves, which means the hook will always return undefined. The promise should either resolve with a real implementation or use a different approach to initialize the data.
| () => new Promise<BetaABTestAPI>(() => {}), | |
| () => undefined, |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
We want it to never resolve as it will be replaced.
There was a problem hiding this comment.
This was copied from the current client useAB added in #4200, it is initially set to a promise that won't resolve, when SetABTests.importable runs it sets it to the initialised ab test API.
It needs to be a promise, not sure if it necessarily needs to be a promise that doesn't resolve. Promise.resolve(undefined) might do the trick too.
There was a problem hiding this comment.
Is it possible to leave a comment explaining that? If you will keep it unresolved or even if you resolve it with undefined a comment would be good.
| /** | ||
| * The 'abTests' is for use by external scripts that need to | ||
| * access A/B test information. Do not use this directly | ||
| * in DCR, instead use the `useAB` hook as it is csr/ssr aware. | ||
| */ |
dotcom-rendering/src/lib/useAB.ts
Outdated
| export const useBetaAB = (): BetaABTestAPI | undefined => { | ||
| const { data } = useSWRImmutable( | ||
| 'beta-ab-tests', | ||
| () => new Promise<BetaABTestAPI>(() => {}), |
Yup good idea 👍 |
github-actions
bot
removed
the
run_chromatic
Co-authored-by: Imogen Hardy [email protected]
What does this change?
This is based off @i-hardy's work that was integrated into our proof of concept branch for the new framework.
Adds a "beta" client for detecting if a user is in a test and/or variant, so that behaviour/rendering can be changed if a user is in said test/variant.
Client side tests are read from the
gu_client_ab_testscookie, and server side tests read fromserverSideABTestson the data from frontend/window.There's a react hook for use in DCR, as well as a method on the window for other bundles such as commercial to use. The hook works both client and server side, when running client side it will return both server and client side tests, but when used server side will only return server side tests.
Why?
As all test participation is calculated at the edge, the current frameworks use of
@guardian/ab-coreis no longer needed, with the Ophan reporting integrated here in DCR.We've created a new "beta" client so that we can compare the old and new frameworks behaviour.